I noticed unreachable code in rrsync. Instead of just cleaning it up, I thought I would also add a nice new feature.

Clean up

Line 305:

    arg = arg.lstrip('/')
    if args.dir != '/':
        if HAS_DOT_DOT_RE.search(arg):
            die("do not use .. in", opt, "(anchor the path at the root of your restricted dir)")
        if arg.startswith('/'):
            arg = args.dir + arg

The last 2 lines are never called because arg.lstrip('/') ensure the condition is never true (lstrip removes multiple instances if needed)

New feature: motivation

rsync supports only relative path. This is good for security, it avoids shenanigans with symlinks or other prefix tricks. However, this is not convenient to the user, because the client needs to be aware of what the DIR argument for rrsync is. It means that in some sense rrsync makes it not backward compatible.

If I have all my client code running rsync, and I decide later to add rrsync to increase security, then I have to modify all my client code to change relative paths instead.

If even later I decide to change DIR, to a more or less restricted entry, then again I have to modify all my client code.

This is not logical because the clients should only be aware of what they want to rsync, they should not have to know what restrictions are imposed on them

New feature: implementation

I tried to minimize the number of lines of code, so you (mainteners) probably don't need too much of an explanation. I'll just mention that I tried to make sure my patch was as secure as possible. The main mechanism is that I am not changing the logic. Even when we pass an absolute path, rrsync still passes the relative path to rsync. So on the server I am NOT actually resolving the absolute path (it's still invoked exactly like in the current version). That makes it resistant to trying to escape the rrsync DIR, no symlink escape, no prefix trick.

I also wanted to make sure this is 100% backward compatible. It would have been natural to just check if the given path is an absolute path. However, the current behaviour of rrsync is to append a given absolute path to the DIR. I would argue absolute path should be respected, and rrsync should actually error out in that scenario. However, I suspect you might not want to change that behaviour, so I just added a new argument.

New feature: usage

On the server:
ForceCommand rrsync -absolute /path/to/restricted/

On the client
rsync user@host:/path/to/restricted/dir .

Let me know what you think. Happy to make any change as you see fit, if that helps.

Testing

I tested on my end on OpenBSD and macOS, and it worked correctly.